Repository navigation
chore(ci): stop blocking django migrations next to nodejs changes - #130701
Conversation
The same-PR rule never enforced the order it was meant to protect. It reads each PR's own diff, so a migration layer stacked under a Node layer passes on both, and the queue lands the stack in one go. Two separate PRs merged minutes apart pass too. The check is also not a required status. The Node deploy hook now holds every Node rollout in prod until the Django migrations its image needs are applied, which covers one PR, a stack, and a batch alike. With it, a migration and its Node reader in one PR is the safe shape, not the risky one. - drop the Django + nodejs rule - keep ClickHouse + nodejs, split out of the old migrations filter, since the hook does not check ClickHouse - keep sqlx + nodejs and sqlx + rust services, since sqlx is shadow-only and Rust has no hook - point the docs and the error text at the hook, and say not to stack the remaining splits
|
😎 Merged successfully - details. |
|
@codex review |
|
Risk: No findings This PR removes the CI rule that forced Django migrations and nodejs/ changes into separate PRs, keeping the ClickHouse and sqlx separation rules, and relies on the existing Node deploy hook (nodejs/src/schema-check/, baked into Dockerfile.node) to hold Node rollouts until required Django migrations are applied. I verified the hook exists, is wired into the image build, and that the renamed CI filter and its env variables are internally consistent. No security-relevant surface is touched. Sentinel reviewed |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🤖 CI report
|
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…ango-node-separation-rule
Any edit to .github/scripts/trunk-impacted-targets.js puts the PR in the universal Trunk lane, so it cannot merge in parallel. The comment there still names Django migrations and is fixed in a later PR that edits the script.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
The CI change enters risky territory, with assurance supplied by the author's owning-team membership. No substantive review concerns or policy-listed refusal grounds are present; the discussion-only bot summaries are not top-level reviews.
- Author wrote 9% of the modified lines and has 50 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from chatgpt-codex-connector[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 40L, 1F substantive, 48L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (48L, 2F, two-areas, chore) |
| stamphog 2.7.0 | .stamphog/policy.yml @ e929092 · reviewed head e929092 |
Problem
Changes
nodejs/nodejs/nodejs/How did you test this code?
nodejs/changes.Test rationale: a test would only restate the path filters.
Release status
Docs update
docs/published/handbook/engineering/safe-django-migrations.md, section 5.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Opus 5.5 (
claude-opus-5-5)Skills:
/authoring-ci-workflows,/writing-code-comments,/writing-pr-descriptions,/babysit-prMore context for AI agents
migrationsfilter so the ClickHouse + nodejs rule survives.trunk-impacted-targets.jscomment edit was reverted because it forced the universal lane.trunk-impacted-targets.jsstill says Django; any edit there puts a PR in the universal lane, so it waits for a PR that edits that script.ci-migrations-service-separation-check.yml.